CONTRIBUTING I have changed line 35 of eslintrc.js based on the issue #313
#839
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to the JSON Schema Community. Thanks a lot for creating your first pull request!! 🎉🎉 We are so excited you are here! We hope this is only the first of many! For more details check out README.md file.
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
|
Why do we think an ADR is required for this? It seems like such a small thing. |
Thank you for asking. I believe the problem was resolved through issue #313 . However it has not been changed in the source code. That affects the set up on Windows and very likely on Unix computers. The line shall be changed so that the lint tests go through along with the cypress tests. Besides, the installation guide may mention that Visual Studio Code is the IDE which matches with the project. Afterwards, if a pull request has an issue associated with it, it should inherit present ad-required labels.(CONTRIBUTING.md) |
|
ADRs are for (generally large and impactful) topics which have been thoroughly discussed and a vote or some other mechanism has been utilized to make a final decision. The ADR then documents that decision. I don't think that line terminations in the website code fit that requirement. Why not update CONTRIBUTING as part of this PR? You're making the change here, we should go ahead and update the instructions to others. It should also be part of the build so that the build fails if the code's not correct (if it's not doing that already). |
AymarN
changed the title
adr-required I have changed line 35 of eslintrc.js based on the issue #313CONTRIBUTING I have changed line 35 of eslintrc.js based on the issue #313
Thank you for your recommendation. Are there any blockers to merge ? |
|
This needs someone more familiar with the build tools than me. cc: @jdesrosiers / @benjagm |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #839 +/- ##
========================================
Coverage ? 100.00%
========================================
Files ? 1
Lines ? 20
Branches ? 12
========================================
Hits ? 20
Misses ? 0
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Thank you. I updated the branch and hoping for your approval. |
|
Congratulations, @AymarN for your first pull request merge in this repository! 🎉🎉. Thanks for your contribution to JSON Schema! |
|
Congrats on merging your first Pull Request!! |
What kind of change does this PR introduce?
The line 35 of eslintrc has been changed.
826:
Screenshots/videos:
If relevant, did you update the documentation?
Summary
There is a set up issue on windows once the installation guide is followed. An error is generated:
Expected linebreaks to be 'LF' but found 'CRLF' On Windows.
313
#826
Does this PR introduce a breaking change?
That affects the set up on Unix computers and on Windows.